Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor topic_tools to use app-based settings and remove unused ASSE… #4095

Merged

Conversation

benjaoming
Copy link
Contributor

We found a couple of early blockers before 0.14 is released. This was supposed to be schedule for 0.15, but it's hard to fix without solving parts of #4054.

@benjaoming benjaoming added this to the 0.14.0 milestone Jul 14, 2015
@benjaoming benjaoming self-assigned this Jul 14, 2015
This is the path that KA Lite will use to look for KA Lite video files to play. Change the path to another local directory to get video files from that directory. NB! Directory has to be writable for the user running the server in order to download videos.
* ASSESSMENT_ITEMS_DATABASE_PATH = "<path to desired content folder>" (default=~/.kalite/content/assessmentitems.sqlite)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -8,17 +8,18 @@
from multiprocessing.dummy import Pool as ThreadPool
from threading import Lock

from django.conf import settings
from django.conf import settings as django_settings

This comment was marked as spam.

This comment was marked as spam.

@benjaoming
Copy link
Contributor Author

Going to work on @jamalex 's comment about settings.DATABASES being initialized from a default ASSESSMENT_ITEM_DATABASE_PATH

@benjaoming
Copy link
Contributor Author

The easy solution seems to be that we should not let the ASSESSMENT_ITEM_DATABASE_PATH be configurable for now.

It's an odd case: We want CONTENT_ROOT to be configurable for serving media, yet we need a database connection established on the basis of where CONTENT_ROOT points. I would suggest to leave it out of the docs.

In a longer perspective, things like settings.DATABASES should always be configured through ENVIRONMENT, it's a suitable "high-level" approach for this specific setting and that would also follow from normal conventions.

We could mention settings.DATABASES in the docs instead, but it's kind of a heavy chapter and people who want to mess with it are better off reading the source code and the django documentation.

@benjaoming
Copy link
Contributor Author

Another issue regarding this is about the system-wide assessment items.... they might mean that the database is in a different location, forcing this logic to be pushed back the settings.base :(

@MCGallaspy
Copy link
Contributor

Okay, so do assessment items count as user data or application data? In one sense they are application data since they are associated with a version, but in another sense they are user data since they can be downloaded after installation and their location on disk is configurable. (Though this is not the case in Windows! where assessment items are bundled with the installer.)

What are the likely cases where the end-user will install ka-lite and download the assessment items separately?

@benjaoming
Copy link
Contributor Author

@MCGallaspy
Yes, to summarize our current situation: We offer an assessment-free version (understandably since its 500 MB) where users download the items into their USER_DATA_ROOT. But we also offer bundles where they will be available from a system-wide location.

The most likely cases are:

  1. For devs, we constantly change ka-lite and the assessment items, so we need an easy way around it
  2. Testing: We shouldn't put 500 MB of system data on the test server
  3. Users with online access: They will find it easier to install a light-weight KA Lite and then download the assessment items if they need them - some might just want videos?
  4. THE FUTURE! Assessment items might be available from separate sources.

# MY_SETTING_VAR = 123
from kalite.project.settings.raspberry_pi import *
# Put your settings here, e.g.
# MY_SETTING_VAR = 123

This comment was marked as spam.

This comment was marked as spam.

@benjaoming benjaoming mentioned this pull request Jul 15, 2015
8 tasks
* CONTENT_ROOT = "<path to desired content folder>" (default=ka-lite/content)
This is the path that KA Lite will use to look for KA Lite video files to play. Change the path to another local directory to get video files from that directory. NB! Directory has to be writable for the user running the server in order to download videos.
* TIME_ZONE = <desired time zone> (default = "America/Los_Angeles")
* ``DEBUG = <True or False> (default = False)``

This comment was marked as spam.

This comment was marked as spam.

@benjaoming
Copy link
Contributor Author

This PR is very critical, as there are no one else who have expressed a desire for a review, I'm going to merge it so we can see if it works.

benjaoming added a commit that referenced this pull request Jul 16, 2015
Refactor topic_tools to use app-based settings and remove unused ASSE…
@benjaoming benjaoming merged commit 4efc46e into learningequality:0.14.x Jul 16, 2015
@jamalex jamalex mentioned this pull request Jul 16, 2015
10 tasks
@benjaoming benjaoming deleted the eliminate-derived-settings branch August 19, 2015 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants